New job queue: worker registration and leader election #3307
Conversation
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
76afd6a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://410771fa.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://quenting-new-queue-initial.matrix-authentication-service-docs.pages.dev |
sandhose
force-pushed
the
quenting/new-queue/initial
branch
from
48e5507
to
e419853
Compare
There was a problem hiding this comment.
This seems reasonable, but it also seems quite intricate and would benefit from a careful review, including aspects like whether it's robust against clock drift, node failures, etc — that sort of thing.
I would also want to carefully review what happens whether there are any problems if the current leader loses connection but still believes it is the leader.
| -- The leader is responsible for running maintenance tasks | ||
| CREATE UNLOGGED TABLE queue_leader ( | ||
| -- This makes the row unique | ||
| active BOOLEAN NOT NULL DEFAULT TRUE UNIQUE, |
There was a problem hiding this comment.
I know it sounds silly, but I'd make this a PRIMARY KEY — maybe this sounds dogmatic? But there a handful of tools are not happy with tables that don't have a primary key, e.g. logical replication in Postgres by default, I'd say it's worth always using it instead of UNIQUE etc.
| // If no row was updated, the worker was shutdown so we return an error | ||
| DatabaseError::ensure_affected_rows(&res, 1)?; | ||
|
|
||
| Ok(worker) |
There was a problem hiding this comment.
the docstring says this returns the modified worker, but I don't see us modifying it.
I would expect the worker to track its own validity timestamps, but I guess the critical thing here is just that we 'take away' the Worker if we can't renew it?
| clock: &dyn Clock, | ||
| threshold: Duration, | ||
| ) -> Result<(), Self::Error> { | ||
| let now = clock.now(); |
There was a problem hiding this comment.
is it reasonable to rely on the system clock (which could drift between servers)?
I suppose we could use the Postgres database's clock alternatively. But I don't know which one is best, mostly just interested in considering it carefully
There was a problem hiding this comment.
That's a fair point, although I'd imagine multiple servers in the same datacenter usually have the same time source/NTP server?
The clock is abstracted through a trait though, so maybe at some point we can take into account time drift, and regularly sync the local system clock with the database or something, but I wouldn't worry too much about it for now
sandhose
force-pushed
the
quenting/new-queue/initial
branch
from
e419853
to
1370a04
Compare
sandhose
force-pushed
the
quenting/new-queue/initial
branch
from
2f19fff
to
80aa6fa
Compare
sandhose
force-pushed
the
quenting/new-queue/initial
branch
from
80aa6fa
to
f060abe
Compare
We would like to use the underlying connection from the PgListener, which was added in a patch, but not yet merged or released.
sandhose
force-pushed
the
quenting/new-queue/initial
branch
from
f060abe
to
76afd6a
Compare
This adds the base for the new job queue system, with a simple worker registration system, as well as a leader election system.
The worker registration is meant to be used to detect lost workers and reschedule dead tasks they locked.
The leader election system is meant to have one leader performing all the maintenance work, like rescheduling tasks.
Part of #2785